Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Julia bindings #1025

Open
wants to merge 111 commits into
base: dev
Choose a base branch
from
Open

Julia bindings #1025

wants to merge 111 commits into from

Conversation

eschnett
Copy link
Contributor

@eschnett eschnett commented Jun 27, 2021 •
edited
Loading

This adds bindings for the Julia language to openPMD. These bindings are still a work in progress; they are incomplete and/or inefficient.

Julia bindings consist of two parts:

  • A C++ wrapper that defines the functions that can be called from Julia
  • A Julia module that exports these functions in a way that looks "natural" for Julia

Since Julia is a different language, one probably should not wrap all features one-to-one. For example, types are first class objects in Julia, so that much of the Datatype class is not necessary. Also, Julia is not object-oriented, and thus all function are free functions (and not member functions), etc.

I'm posting this here in case someone wants to add a comment.

Related to #625

To Do

  • add example(s)/tests
  • add to CI
  • document requirements in README and docs
  • discuss packaging plans / continuous delivery options

@ax3l ax3l requested review from ax3l and williamfgc June 28, 2021 06:54
@ax3l ax3l added frontend: Julia api: new additions to the API labels Jun 28, 2021
ax3l
ax3l reviewed Jun 28, 2021
View reviewed changes
CMakeLists.txt Outdated
if(openPMD_HAVE_JlCxx)
get_target_property(JlCxx_location JlCxx::cxxwrap_julia LOCATION)
get_filename_component(JlCxx_location ${JlCxx_location} DIRECTORY)
set(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib;${JlCxx_location}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memo to myself: generalize.

  • Control RPATH install (default: on) via option.
  • Add JlCxx_location only to openPMD_jl target

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prepared in #1105

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have pushed a commit adapting to this change d427b19
Would probably be good if you look at that again @ax3l @eschnett

@ax3l
Copy link
Member

ax3l commented Jun 28, 2021

That is really cool. Wuhu!

Let me ping potentially @williamfgc, @berceanu and @MaxThevenet as they are excited about Julia as well. Maybe one of you is interested to add a review? :) (Otherwise sorry for the noise, please don't hesitate to unsubscribe from the thread.)

@eschnett
Copy link
Contributor Author

Regarding packaging:

  • Almost all Julia packages are available from their package registry. The Julia package should be split out from this repository and be published there, so that it can be installed in the Julia-usual way. I have begun to move it here.
  • Binary dependencies for Julia packages are often published on Yggdrasil. This is in principle a generic package manager, but it's mostly used for Julia. The one defining feature of Yggdrasil is that it delivers binaries that are very quick to install. I have already created an openPMD-api package there. This package does not yet have Julia bindings, though.

I think the best path forward would be to release openPMD-api binaries on Yggdrasil, with the Julia bindings enabled, and to use it from the openPMD.jl Julia package. I'm working on this at the moment.

Regarding examples and tests: Tests are there in the Julia package. Examples are completely missing.

Regarding docs: Simple docs are there (the API is documented). There is no introduction or overview. The general approach of how the C++ functions are mapped to Julia functions is also not yet described.

Regarding CI: The Julia package will have CI, once it's set up properly.

@williamfgc
Copy link

@ax3l @eschnett thanks for the great work here. I'll probably be mostly learning since @eschnett did all the things I was planning to use in another project. Just a few things to think about that are collaterals:

  • Julia allows pre-compilation for first-time use heavy package with PackageCompiler.jl. Just something to consider if it becomes a problem.
  • Jupyter, since NERSC and OLCF have JupyterHub services, the bindings can be tested on both platforms for post-processing. For OLCF some pre-configuration is needed to get Julia going, there is a v1.4 module on Summit, there is a bunch of bug-fixes and improvements with Julia v1.6 (Now v1.7 I believe). Something to consider to establish a minimal version of Julia that is supported.
  • Since Python and Julia bindings are both "lightweight" layers on top of C++ (in the runtime sense, this is a great effort). I'd look into performance penalties, if any.

@eschnett for the C++-object to Julia-type wrapper are you just translatin foo.bar(...) into bar(foo, ..)?

@eschnett
Copy link
Contributor Author

@williamfgc Performance isn't good yet. At the moment, I'm copying data in some places where I really should pass references. This is just because I'm fighting other battles at the moment. I've marked most of the places where performance is bad with TODO.

Yes. foo.bar(...) becomes bar(foo, ...). I've taken the liberty to make a few other changes as well:

  • fooBar becomes foo_bar (it's a Julia convention to use lower_case for function names)
  • certain functions have no standard name in C++, but have a standard name in Julia. If so, I use these names (e.g. getDimensionality -> ndims, extent -> size, dtype -> eltype)
  • setPosition becomes set_position!, with a trailing exclamation mark
  • Small arrays (e.g. Extent) become tuples in Julia
  • C++ just loves using different integer types for different purposes. These mostly become just Int in Julia.

I want the code to look "natural" in Julia.

@eschnett eschnett mentioned this pull request Jul 3, 2021
Closed
@franzpoeschel
Copy link
Contributor

I have pushed three commits:

  1. Merged the current dev
  2. Set some CMake properties for libopenPMD.jl.so
  3. Added a minimal lowlevel test (I will probably expand this one a bit, just want to see what the CI thinks of it)

@franzpoeschel franzpoeschel force-pushed the eschnett/julia-bindings branch 3 times, most recently from 7379a6d to 39cb4cf Compare August 11, 2023 12:15
@franzpoeschel
Copy link
Contributor

I hadn't noticed that you added the minimal lowlevel example, too. I've merged them both now, they now run from within ctest, and the test script is inside the test directory now.

I will expand the test a little and have a more in-depth look at your changes next week.

franzpoeschel
franzpoeschel reviewed Aug 18, 2023
View reviewed changes
docs/source/install/install.rst

# We need Julia 1.7 or newer
using("Pkg")
Pkg.add("openPMD")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only remaining question before merging this PR is how we should go about documentation?
I feel like we should mark this as experimental until we also have the high-level bindings merged?
Should we add documentation apart from install.rst, too? I think that we can keep it short due to the experimental status.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds fine. I would point to openPMD.jl as an existing package that uses these bindings, so that people are not put off by the experimental status – it's not openPMD that is experimental, it's only the way things are currently glued together.

@eschnett
Merge branch 'eschnett/julia-bindings' of ssh://github.com/eschnett/o…
184610f 
…penPMD-api into eschnett/julia-bindings
@eschnett
Don't export openPMD_datatypes to Julia
d59773a
@eschnett
Merge branch 'dev' into eschnett/julia-bindings
724806e 
* dev:
  Fix CMake: HDF5 Libs are PUBLIC (openPMD#1520)
  Fix `chmod` in `download_samples.sh` (openPMD#1518)
  CI: Old CTest (openPMD#1519)
  Python: Fix ODR Violation (openPMD#1521)
  replace extent in weighting and displacement (openPMD#1510)
  CMake: Warn and Continue on Empty HDF5_VERSION (openPMD#1512)
  Replace openPMD_Datatypes global with function (openPMD#1509)
  Streaming examples: Set WAN as default transport (openPMD#1511)
  TOML Backend (openPMD#1436)
  make it possible to manually set chunks when loading dask arrays (openPMD#1477)
  [pre-commit.ci] pre-commit autoupdate (openPMD#1504)
  Optional debugging output for AbstractIOHandlerImpl::flush() (openPMD#1495)
  Python: 3.8+ (openPMD#1502)

# Conflicts:
#	.github/workflows/linux.yml
#	src/binding/python/Series.cpp
@eschnett
Don't export openPMD_datatypes to Julia
54a5052
@eschnett
Configure Julia include and library directories
fa2efa5
@eschnett
Configure Julia include and library directories
5d5612d
@eschnett
Don't configure Julia include and library directories
9eaec53
@eschnett
Julia: Correct set_const definitions for Datatype enum
cac9488
@eschnett
CI: Correct shared library suffix for Apple
5213e2e
@eschnett
Julia: Correct low-level test
805756f
@eschnett
CI: Install libcxxwrap-julia @v0.11.0
a0b015e
@eschnett
Merge branch 'dev' into eschnett/julia-bindings
26b6ca7 
* dev:
  [pre-commit.ci] pre-commit autoupdate (openPMD#1536)
  Fix double mesh.read() call (openPMD#1535)
  Python 3.12: Remove Distutils (openPMD#1508)
  openpmd-pipe: fix handling of constant components (openPMD#1530)
  Bug fix: Linear read mode cannot directly access specific file of file-based Series (openPMD#1533)
  Unknown openPMD version in data: Add upgrade hint (openPMD#1528)
@eschnett
Copy link
Contributor Author

I have grown quite dissatisfied with libcxxwrap_julia. This library is not distributed with Julia, yet it depends on the current Julia version in a fragile way. I have not managed to build a version of the openPMD Julia bindings that reliably work with beta releases or with the development version of Julia. It seems that each new Julia release will require updating the bindings.

At the same time, the bindings produced by this library are not directly usable from Julia. The API it creates does not look "natural" for Julia, and requires another wrapper (written in Julia) to make it usable.

Thus #1537. This creates C bindings for openPMD, and wrapping them from Julia will be about the same amount of work as using libcxxwrap_julia. This approach has worked well for ADIOS2, without any headaches. (Of course there needs to be a C API, and it needs to be maintained, but that's not terribly difficult.)

@berceanu berceanu mentioned this pull request Oct 16, 2023
Open
@franzpoeschel franzpoeschel mentioned this pull request Oct 17, 2023
Draft
@ax3l ax3l modified the milestones: 0.16.0, 0.17.0 Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@franzpoeschel franzpoeschel franzpoeschel left review comments

@ax3l ax3l ax3l left review comments

@williamfgc williamfgc Awaiting requested review from williamfgc

Assignees
No one assigned
Labels
api: new additions to the API frontend: Julia
Projects
None yet
Milestone
0.17.0
Development

Successfully merging this pull request may close these issues.
4 participants
@eschnett @ax3l @williamfgc @franzpoeschel